-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(exporters)!: rewrite exporter config logic #4971
feat(exporters)!: rewrite exporter config logic #4971
Conversation
2b185dc
to
af05548
Compare
9bfaad5
to
b93d9db
Compare
11b5f20
to
3c1756f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a big one :)
I like the approach to have config separated by functions and the merge the results. Just a couple of questions before approving it. Thanks for taking the time to simplify the config
import { getSharedConfigurationFromEnvironment } from './shared-env-configuration'; | ||
import { OtlpHttpConfiguration } from './otlp-http-configuration'; | ||
|
||
function getHeadersFromEnv(signalIdentifier: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: do you think signals are going to grow much in the future that we need to have the identifier as a string
type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't think they will grow much. I mostly just chose string as it was the easiest choice to work with and it does not require us to export a new type that could be difficult to extend later.
Do you think we should have a SignalIdentifier
type? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one might think is a commodity but with the type the editor helps you to avoid passing a string which does not refer to any Signal. Is not wrong to not have a type here. I'll leave it to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will leave it as-is - maybe I'm traumatized by making unintentional breaking changes in TypeScript libraries, but I feel like whatever type I can come up with won't be extendable in a safe way later on. 😕
The fact that libraries have to be type-compatible with other versions of themselves is kind of a mind-bender that did not exist in other languages I came from originally. This API will mostly be internal, so I'll opt to leave it like this for now. We can re-visit this choice in a later iteration - I'm planning to have a final round of API review when the whole exporter restructuring is over to work out smaller things like this. 🙂
if (!url.endsWith('/')) { | ||
url = url + '/'; | ||
} | ||
return url + path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is not checking that
url
does not already contain a path- the result string is a valid URL, path could contain invalid chars
So the following call is possible
appendResourcePathToUrl('http://foo.com/bar/test.txt', 'v1/logs')
// returns 'http://foo.com/bar/test.txt/v1/logs'
I guess we should let the users define the URL they want but I think we should diag.warn
or diag.debug
the result for debug purposes (please ignore this comment if the diag.
is set in a file below, I haven't reached them yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point about checking that the final URL is valid. 👍 I added additional checks and logs to that function. I settled on warn
as it's very likely that it'll not be able to continue with an invalid URL. 5d0aae1
It's actually intended to append if it does not contain a path already - this is specified behavior for the OTEL_EXPORTER_OTLP_ENDPOINT
env var where this function is used. See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#endpoint-urls-for-otlphttp
For signal-specific ones like OTEL_EXPORTER_OTLP_ENDPOINT
no path is actually appended - just the root path if none is there already that's the function above this one 🙂
); | ||
assert.deepStrictEqual(config.headers, { | ||
foo: 'foo-user-provided', | ||
baz: 'null', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bar: undefined
does not appear in the result but baz: null
does and it's stringified. Any reason for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is mostly to keep the same behavior of the current implementation, which also handles it this way 🙂
undefined
bar: undefined
is dropped as calls with http
module will throw when there's an undefined
header value:
> const http = require('http');
undefined
> http.get({host: 'example.com', headers: {'foo': undefined}})
Uncaught:
TypeError [ERR_HTTP_INVALID_HEADER_VALUE]: Invalid value "undefined" for header "foo"
at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)
at new NodeError (node:internal/errors:405:5)
at __node_internal_ (node:_http_outgoing:624:11)
at ClientRequest.setHeader (node:_http_outgoing:651:3)
at new ClientRequest (node:_http_client:291:14)
at request (node:http:100:10)
at Object.get (node:http:111:15) {
code: 'ERR_HTTP_INVALID_HEADER_VALUE'
}
We've had users in the past that have ended up with undefined
header values as they set a header to process.env.MY_HEADER_ENV_VAR
, did not set that env var and then ended up with an exporter that would constantly fail.
null
null
is stringified as to ensure it matches the type for Record<string, string>
for the headers. Plus it gets stringified to null
anyway when it's sent via with the http
module so we make it happen earlier to make the type a bit more simple to work with down the chain as we don't have to worry about null values. It's also the current behavior of the exporters, so I tried not to change it.
You can try sending it to https://webhook.site to see that it ends up the same:
> const https = require('https');
undefined
> https.get({host: 'webhook.site', headers: {'foo': null}, path: '/<your-webhook-site-path>'})
> https.get({host: 'webhook.site', headers: {'foo': 'null'}, path: '/<your-webhook-site-path>'})
… 0.54.2 (#7) ![snyk-top-banner](https://redirect.github.com/andygongea/OWASP-Benchmark/assets/818805/c518c423-16fe-447e-b67f-ad5a49b5d123) <h3>Snyk has created this PR to upgrade @opentelemetry/exporter-trace-otlp-http from 0.54.0 to 0.54.2.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **2 versions** ahead of your current version. - The recommended version was released on **a month ago**. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>@opentelemetry/exporter-trace-otlp-http</b></summary> <ul> <li> <b>0.54.2</b> - <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.54.2">2024-11-07</a></br><h2>0.54.2</h2> <h3>🐛 (Bug Fix)</h3> <ul> <li>fix(instrumentation): Fix wrapping ESM files with absolute path <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5094" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5094/hovercard">#5094</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/serkan-ozal/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/serkan-ozal">@ serkan-ozal</a></li> </ul> </li> <li> <b>0.54.1</b> - <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.54.1">2024-11-05</a></br><h2>0.54.1</h2> <h3>🐛 (Bug Fix)</h3> <ul> <li>fix(instrumentation-http): skip malformed forwarded headers. <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/issues/5095" data-hovercard-type="issue" data-hovercard-url="/open-telemetry/opentelemetry-js/issues/5095/hovercard">#5095</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pmlanger/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pmlanger">@ pmlanger</a></li> </ul> </li> <li> <b>0.54.0</b> - <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.54.0">2024-10-23</a></br><h2>0.54.0</h2> <h3>💥 Breaking Change</h3> <ul> <li>feat(exporter-<em>-otlp-</em>)!: rewrite exporter config logic for testability <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4971" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4971/hovercard">#4971</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a> <ul> <li>(user-facing) <code>getDefaultUrl</code> was intended for internal use has been removed from all exporters</li> <li>(user-facing) <code>getUrlFromConfig</code> was intended for internal use and has been removed from all exporters</li> <li>(user-facing) <code>hostname</code> was intended for internal use and has been removed from all exporters</li> <li>(user-facing) <code>url</code> was intended for internal use and has been removed from all exporters</li> <li>(user-facing) <code>timeoutMillis</code> was intended for internal use and has been removed from all exporters</li> <li>(user-facing) <code>onInit</code> was intended for internal use and has been removed from all exporters</li> <li>(user-facing) OTLP exporter config <code>headers</code> type changed from <code>Partial<Record<string, unknown>></code> to <code>Record<string, string></code></li> </ul> </li> <li>feat(otlp-exporter-base)!: do not export functions that are intended for internal use <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4971" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4971/hovercard">#4971</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a> <ul> <li>Drops the following functions and types that were intended for internal use from the package exports: <ul> <li><code>parseHeaders</code></li> <li><code>appendResourcePathToUrl</code></li> <li><code>appendResourcePathToUrlIfNeeded</code></li> <li><code>configureExporterTimeout</code></li> <li><code>invalidTimeout</code></li> </ul> </li> </ul> </li> <li>feat(instrumentation-http)!: remove long deprecated options <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5085" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5085/hovercard">#5085</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a> <ul> <li><code>ignoreIncomingPaths</code> has been removed, use the more versatile <code>ignoreIncomingRequestHook</code> instead.</li> <li><code>ignoreOutgoingUrls</code> has been removed, use the more versatile <code>ignoreOutgoingRequestHook</code> instead.</li> <li><code>isIgnored</code> utility function was intended for internal use and has been removed without replacement.</li> </ul> </li> </ul> <h3>🚀 (Enhancement)</h3> <ul> <li>feat(api-logs): Add delegating no-op logger provider <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4861" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4861/hovercard">#4861</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/hectorhdzg/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/hectorhdzg">@ hectorhdzg</a></li> <li>feat(instrumentation-http): Add support for <a href="https://redirect.github.com/open-telemetry/semantic-conventions/releases/tag/v1.27.0">Semantic Conventions 1.27+</a> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4940" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4940/hovercard">#4940</a> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4978" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4978/hovercard">#4978</a> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5026" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5026/hovercard">#5026</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/dyladan/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/dyladan">@ dyladan</a> <ul> <li>Applies to client and server spans and metrics</li> <li>Generate spans and metrics compliant with Semantic Conventions 1.27+ when <code>OTEL_SEMCONV_STABILITY_OPT_IN</code> contains <code>http</code> or <code>http/dup</code></li> <li>Generate spans and metrics backwards compatible with previous attributes when <code>OTEL_SEMCONV_STABILITY_OPT_IN</code> contains <code>http/dup</code> or DOES NOT contain <code>http</code></li> </ul> </li> </ul> <h3>🐛 (Bug Fix)</h3> <ul> <li>fix(sampler-jaeger-remote): fixes an issue where package could emit unhandled promise rejections <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/Just-Sieb/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/Just-Sieb">@ Just-Sieb</a></li> <li>fix(otlp-grpc-exporter-base): default compression to <code>'none'</code> if env vars <code>OTEL_EXPORTER_OTLP_TRACES_COMPRESSION</code> and <code>OTEL_EXPORTER_OTLP_COMPRESSION</code> are falsy <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/sjvans/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/sjvans">@ sjvans</a></li> <li>fix(sdk-events): remove devDependencies to old <code>@ opentelemetry/[email protected]</code>, <code>@ opentelemetry/[email protected]</code> packages <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5013" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5013/hovercard">#5013</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a></li> <li>fix(sdk-logs): remove devDependencies to old <code>@ opentelemetry/[email protected]</code> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5013" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5013/hovercard">#5013</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a></li> <li>fix(sdk-logs): align LogRecord#setAttribute type with types from <code>@ opentelemetry/[email protected]</code> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5013" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5013/hovercard">#5013</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a></li> <li>fix(exporter-<em>-otlp-</em>): fixes a bug where signal-specific environment variables would not be applied and the trace-specific one was used instead <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4971" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4971/hovercard">#4971</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/pichlermarc/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/pichlermarc">@ pichlermarc</a> <ul> <li>Fixes: <ul> <li><code>OTEL_EXPORTER_OTLP_METRICS_COMPRESSION</code></li> <li><code>OTEL_EXPORTER_OTLP_LOGS_COMPRESSION</code></li> <li><code>OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE</code></li> <li><code>OTEL_EXPORTER_OTLP_LOGS_CLIENT_CERTIFICATE</code></li> <li><code>OTEL_EXPORTER_OTLP_METRICS_CLIENT_KEY</code></li> <li><code>OTEL_EXPORTER_OTLP_LOGS_CLIENT_KEY</code></li> <li><code>OTEL_EXPORTER_OTLP_METRICS_INSECURE</code></li> <li><code>OTEL_EXPORTER_OTLP_LOGS_INSECURE</code></li> </ul> </li> </ul> </li> <li>fix(sdk-node): use warn instead of error on unknown OTEL_NODE_RESOURCE_DETECTORS values <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5034" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5034/hovercard">#5034</a></li> <li>fix(exporter-logs-otlp-proto): Use correct config type in Node constructor</li> <li>fix(instrumentation-http): Fix instrumentation of <code>http.get</code>, <code>http.request</code>, <code>https.get</code>, and <code>https.request</code> when used from ESM code and imported via the <code>import defaultExport from 'http'</code> style. <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/issues/5024" data-hovercard-type="issue" data-hovercard-url="/open-telemetry/opentelemetry-js/issues/5024/hovercard">#5024</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/trentm/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/trentm">@ trentm</a></li> </ul> <h3>🏠 (Internal)</h3> <ul> <li> <p>refactor(exporter-prometheus): replace <code>MetricAttributes</code> and <code>MetricAttributeValues</code> with <code>Attributes</code> and <code>AttributeValues</code> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/4993" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/4993/hovercard">#4993</a></p> </li> <li> <p>refactor(browser-detector): replace <code>ResourceAttributes</code> with <code>Attributes</code> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5004" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5004/hovercard">#5004</a></p> </li> <li> <p>refactor(sdk-logs): replace <code>ResourceAttributes</code> with <code>Attributes</code> <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/pull/5005" data-hovercard-type="pull_request" data-hovercard-url="/open-telemetry/opentelemetry-js/pull/5005/hovercard">#5005</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/david-luna/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/david-luna">@ david-luna</a></p> </li> </ul> </li> </ul> from <a href="https://redirect.github.com/open-telemetry/opentelemetry-js/releases">@opentelemetry/exporter-trace-otlp-http GitHub release notes</a> </details> </details> --- > [!IMPORTANT] > > - Check the changes in this PR to ensure they won't cause issues with your project. > - This PR was automatically created by Snyk using the credentials of a real user. --- **Note:** _You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs._ **For more information:** <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiJiMjU2ZTBmZC0yZWE3LTQ3YzgtOTBmZi0yNDUyOWZiNWUyNjIiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6ImIyNTZlMGZkLTJlYTctNDdjOC05MGZmLTI0NTI5ZmI1ZTI2MiJ9fQ==" width="0" height="0"/> > - 🧐 [View latest project report](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source=github&utm_medium=referral&page=upgrade-pr) > - 📜 [Customise PR templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates?utm_source=&utm_content=fix-pr-template) > - 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) > - 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?pkg=@opentelemetry/exporter-trace-otlp-http&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) [//]: # 'snyk:metadata:{"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"@opentelemetry/exporter-trace-otlp-http","from":"0.54.0","to":"0.54.2"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"b256e0fd-2ea7-47c8-90ff-24529fb5e262","prPublicId":"b256e0fd-2ea7-47c8-90ff-24529fb5e262","packageManager":"npm","priorityScoreList":[],"projectPublicId":"12a8a5f5-3e19-438c-8280-eb8f4ee06d17","projectUrl":"https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":2,"publishedDate":"2024-11-07T13:30:06.896Z"},"vulns":[]}'
Which problem is this PR solving?
Config code for exporters (user-provided, defaults, and env config) is currently hard to test individually and is spread/duplicated all over the different signal exporters.
This PR moves the configuration as much as possible into separate files/functions and splits them up into:
Since most config is now moved to base-packages we can remove the in-depth tests from the specific packages. This reduces duplication and makes further changes easier, as they will touch "fewer" files. Since now the configuration-code is not part of the individual exporter-implementations, collapsing the base exporters into one class becomes simpler as we have to worry less about the inheritance structure (abstract methods were removed in this PR as they were config-related).
In addition to these improvements, this PR also fixes #3748, #3747 which was a side-effect of the previous way of implementing env var config.
Breaking changes:
getDefaultUrl
was intended for internal use has been removed from all exportersgetUrlFromConfig
was intended for internal use and has been removed from all exportershostname
was intended for internal use and has been removed from all exportersurl
was intended for internal use and has been removed from all exporterstimeoutMillis
was intended for internal use and has been removed from all exportersonInit
was intended for internal use and has been removed from all exportersparseHeaders
is now not exported anymoreappendResourcePathToUrl
is now not exported anymoreappendResourcePathToUrlIfNeeded
is now not exported anymoreconfigureExporterTimeout
is now not exported anymoreinvalidTimeout
is now not exported anymoreFollow-up steps:
OTEL_EXPORTER_OTLP_TEMPORALITY_PREFERENCE
, this will be done in a follow-up.OTLPExporterNodeBase
,OTLPExporterBrowserBase
,OTLPGrpcExporterNodeBase
into one shared base, that favors composition over inheritance. This can be done by:OTLPExporterDelegate
(draft) based on the "new" config, this replaces theOTLPExporterNodeBase
,OTLPExporterBrowserBase
,OTLPGrpcExporterNodeBase
classes which right now do essentially the same, except for the constructor which will be replaced by the factory function.@opentelemetry/otlp-transformer
, testing that the output is the correct format should suffice)Fixes #3748
Fixes #3747
Type of change
How Has This Been Tested?